netfilter: nf_tables: don't fail inserts if duplicate has expired
authorSasha Levin <sashal@kernel.org>
Fri, 22 Sep 2023 16:30:18 +0000 (18:30 +0200)
committerSalvatore Bonaccorso <carnil@debian.org>
Fri, 29 Sep 2023 04:15:31 +0000 (05:15 +0100)
commit6d39d87bf5654666bd9f479b74912e879b394098
tree6462ca7ef7536710a2ad96f1e387ca47aaf31e00
parentf0ea76e278d3e9f2c01b8deddbd2be9a6474e846
netfilter: nf_tables: don't fail inserts if duplicate has expired

commit 7845914f45f066497ac75b30c50dbc735e84e884 upstream.

nftables selftests fail:
run-tests.sh testcases/sets/0044interval_overlap_0
Expected: 0-2 . 0-3, got:
W: [FAILED]     ./testcases/sets/0044interval_overlap_0: got 1

Insertion must ignore duplicate but expired entries.

Moreover, there is a strange asymmetry in nft_pipapo_activate:

It refetches the current element, whereas the other ->activate callbacks
(bitmap, hash, rhash, rbtree) use elem->priv.
Same for .remove: other set implementations take elem->priv,
nft_pipapo_remove fetches elem->priv, then does a relookup,
remove this.

I suspect this was the reason for the change that prompted the
removal of the expired check in pipapo_get() in the first place,
but skipping exired elements there makes no sense to me, this helper
is used for normal get requests, insertions (duplicate check)
and deactivate callback.

In first two cases expired elements must be skipped.

For ->deactivate(), this gets called for DELSETELEM, so it
seems to me that expired elements should be skipped as well, i.e.
delete request should fail with -ENOENT error.

Fixes: 24138933b97b ("netfilter: nf_tables: don't skip expired elements during walk")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Gbp-Pq: Topic bugfix/all
Gbp-Pq: Name netfilter-nf_tables-don-t-fail-inserts-if-duplicate-.patch
net/netfilter/nft_set_pipapo.c